fix: pagination for null-datetime items and empty fields object#1135
Merged
Conversation
Two pagination defects affecting GET /search and the items endpoints: 1. Items without a `datetime` (only start/end_datetime) broke pagination. OpenSearch sorts missing dates to a Long sentinel that loses precision in JS and 400s when reused as `search_after`. Fix: the default sort now uses `missing: 0` for `properties.datetime`, and the `next` token is base64url-encoded JSON of the sort values (lossless for numbers/nulls) instead of a lossy comma-join. buildSearchAfter still accepts the legacy comma format. (#608, #1082) 2. An empty `fields` object serialized into the GET `next` link as `fields={}`, which on the follow-up request parsed as a field named "{}" and stripped the response _source. Fix: drop empty objects/arrays from next-link params. Adds test-api-pagination.ts covering both, plus a regression for #823 (next link present when the sortby field is excluded — already fixed by the #1046 sort rework). Updates two next-token assertions for the new token format. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes pagination edge cases in the STAC API by making next tokens lossless (base64url JSON of OpenSearch sort values), stabilizing the default datetime sort for missing datetimes, and preventing empty request params (notably fields: {}) from corrupting subsequent-page requests.
Changes:
- Encode
nextas base64url JSON (with legacy comma-token fallback) and adjust default sort to usemissing: 0forproperties.datetime. - Drop empty objects/arrays from pagination link params to avoid
fields={}poisoning page-2 requests. - Add/adjust system tests to cover null-datetime pagination and sortby-excluded pagination behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/lib/database.ts |
Updates default sort and implements base64url JSON next token parsing/encoding for search_after. |
src/lib/api.ts |
Filters empty values out of generated pagination link params to avoid fields={} in next links. |
src/lib/types.ts |
Broadens OpenSearch request typing to allow numeric/null search_after values and optional missing in sort rules. |
tests/system/test-api-search-get.ts |
Updates assertions to decode/verify the new base64url JSON next token format. |
tests/system/test-api-pagination.ts |
Adds system regressions for null-datetime pagination and sortby-excluded pagination. |
CHANGELOG.md |
Documents the pagination fixes. |
Comments suppressed due to low confidence (1)
src/lib/api.ts:364
- For GET pagination links,
fieldshas already been parsed into an object, anddictToURIwill JSON.stringify it. On the follow-up GET request,extractFieldstreatsfieldsas the GET comma syntax (not JSON), so any non-emptyfieldsfilter will be re-parsed into garbage and can strip_sourceon page 2. Convert thefieldsobject back into the GET comma-separated format when building GET next links (but keep the object form for POST link bodies).
const nextParams = pickBy(
assign(parameters, { bbox, intersects, limit, next: lastItemSort, collections, filter }),
isPresent
)
if (httpMethod === 'GET') {
const nextQueryParameters = dictToURI(nextParams)
link.href = `${endpoint}?${nextQueryParameters}`
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per review: `followNext` always issued a GET, but POST /search returns POST-style next links (token in the body, not the href), so the sortby-excluded regression test wasn't actually validating page 2. Follow the link's method, and assert page 2 returns a different item. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
|
I worked with claude to help me understand what this code is for and how it works. It identified a discrepancy between the pairing of buildSort and buidSearchAfter, and wrote a test to confirm a bug was present. I had it create a fix for that bug on the buildSort side; those changes are available for review in #1161. |
A custom `sortby` replaced the default sort's unique `id`/`collection` tiebreakers. Items sharing the user's sort value — worst when the sort field is absent on some items and they all collapse to the same missing-value key — could not be disambiguated by `search_after`, so the follow-up page skipped them. buildSort now appends the `id` tiebreaker the default sort already guarantees, unless the caller already sorts by id. Adds a system test paginating a custom sort where half the items lack the sort field; it fails without the tiebreaker. Also corrects comments (and the CHANGELOG) that credited base64 token encoding with fixing the Long sentinel — the sentinel is lossy before encoding; `missing: 0` on the default sort and the tiebreaker on custom sorts are what actually fix it. Refs #608, #1082
jkeifer
approved these changes
Jul 1, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the pagination bugs tracked for 5.0.1.
1. Null-datetime pagination (#608, #1082)
Items with
datetime: null(onlystart_datetime/end_datetimeset) broke pagination: OpenSearch sorts missing dates to theLong.MINsentinel, which (a) loses precision when parsed in JS and (b) the lossysort.join(',')/split(',')nexttoken corrupted further — the follow-up page 400'd.nexttoken is now base64url-encoded JSON of the sort values (round-trips numbers/nulls losslessly).buildSearchAfterstill accepts the legacy comma format.missing: 0forproperties.datetime, giving missing dates a concrete, reusablesearch_aftervalue instead of the sentinel.2.
fields={}strips the page-2 bodyAn empty
fieldsobject was serialized into the GETnextlink asfields={}, then re-parsed on the follow-up request as a field literally named{}— returning items with an empty_source. Empty objects/arrays are now dropped from next-link params.#823 — already fixed
Confirmed the "next link missing when sortby field excluded" case is already resolved by the 5.0.0 sort rework (#1046); added a regression test so it stays fixed.
Tests
New
tests/system/test-api-pagination.ts(null-datetime traversal + sortby-excluded). Updated twotest-api-search-getassertions for the new token format. Verified: typecheck, lint, unit (69), and the pagination-affected system suite all green.Closes #608
Closes #1082
Closes #823
🤖 Generated with Claude Code